Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add SNI filtering #34

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Add SNI filtering #34

wants to merge 2 commits into from

Conversation

castironclay
Copy link

These changes will allow for filtering off traffic if SNI does not match specified domain. This will allow for improved anonymity of Signal-TLS-Proxy by preventing IP scanners from discovering cert/tls information.

@castironclay
Copy link
Author

any updates on this from code owners?

@castironclay
Copy link
Author

👋

Copy link
Contributor

@jon-signal jon-signal left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hello, and thank you for the contribution!

I think this is a laudable goal, but I think the execution needs some adjustment. In particular, it seems like this approach requires end users to modify the init-certificate.sh script, which would in turn then permanently modify the nginx.conf file for nginx-terminate.

I'd suggest two improvements:

  1. In init-certificate.sh, can we reuse the domains that the user has already entered when generating the certificate?
  2. Rather than modifying a core config file, could we create a separate, "disposable" config file that nginx.conf would then include? That would mean that subsequent runs of init-certificate.sh would overwrite stale domain names as needed.

It's also worth noting that init-certificate.sh supports multiple domain names, and we should make sure we're allowing for multiple domains in the nginx configuration files.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants